Skip to content

Conversation

mdavis36
Copy link
Contributor

@mdavis36 mdavis36 commented Feb 3, 2025

Summary

  • This pull request introduces chai::expt::ManagedSharedPtr, a smart pointer enabling implicit migration of polymorphic types between host and device memory spaces.
  • With ManagedSharedPtr, you can safely copy a base shared pointer and invoke derived class functionality using the device-side v-table.
  • The implementation ensures that implicit copies transfer the correct derived object data between host and device.
  • Reference counting on the host manages GPU memory, automatically releasing device memory when the object is destroyed.

This has been placed under the expt names space per @adayton1's request since much of the implementation is a duplicate of the underlying logic for chai::ManagedArray.

This work is necessary for the Spheral GPU port.

mdavis36 added 30 commits April 3, 2024 23:08
…cussing on make_shared as the only construction option for now.
…edPtrManager copies working correctly with umpire/chai.
…ehavior from non-poly types in ManagedSharedPtr.
… memory map dtor); Rename shared ptr tests to a more appropriate filename.
@mdavis36 mdavis36 force-pushed the feature/ManagedSharedPtr branch from 53b4ea9 to b7d1cf0 Compare August 6, 2025 21:24
@adayton1
Copy link
Member

adayton1 commented Aug 6, 2025

Could you add a new CMake configuration variable? Something like CHAI_ENABLE_EXPERIMENTAL? Then in CMakeLists.txt only add the new files if CHAI_ENABLE_EXPERIMENTAL is on.

@mdavis36
Copy link
Contributor Author

mdavis36 commented Aug 6, 2025

Could you add a new CMake configuration variable? Something like CHAI_ENABLE_EXPERIMENTAL? Then in CMakeLists.txt only add the new files if CHAI_ENABLE_EXPERIMENTAL is on.

Done, the option by default disables tests in all of the CI jobs. We probably want to keep testing experimental features so they aren't broken, wheres the best place to define that flag for Chai's CI pipeline?

@mdavis36 mdavis36 requested a review from adayton1 August 6, 2025 23:09
@adayton1 adayton1 requested review from robinson96 and liu15 August 6, 2025 23:19
// trigger a moveInnerImpl, which expects inner values to be initialized.
template <bool B = std::is_base_of<CHAICopyable, T>::value,
typename std::enable_if<B, int>::type = 0>
CHAI_HOST bool initInner(size_t start = 0)
{
for (size_t i = start; i < m_size/sizeof(T); ++i) {
m_active_base_pointer[i] = nullptr;
new (&m_active_base_pointer[i]) T();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do these changes fix other behaviors?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it allows default construction of more complex nested chai managed types that would be elements of a managed array.

@adayton1 adayton1 requested a review from owens47 August 21, 2025 22:34
@adayton1 adayton1 requested review from ldowen and removed request for owens47 August 21, 2025 22:48
Copy link
Contributor

@robinson96 robinson96 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@adayton1 adayton1 merged commit b7babdc into develop Aug 22, 2025
7 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants